fix(geo-features): searching via name should include project-id #572
Conversation
|
This pull request is being automatically deployed with Vercel (learn more). marxan – ./app🔍 Inspect: https://vercel.com/vizzuality1/marxan/7vmNyzff4we5d11tfPvJeKd7bKbP marxan-storybook – ./app🔍 Inspect: https://vercel.com/vizzuality1/marxan-storybook/CYjReEzZq4S3UPDupeuvqq5NqGT2 |
kgajowy
force-pushed
the
fix/MARXAN-859-search-feature-query
branch
from
5e961d8
to
77ea909
Compare
kgajowy
force-pushed
the
fix/MARXAN-859-search-feature-query
branch
from
77ea909
to
b388222
Compare
|
Need to rewrite specs. |
There was a problem hiding this comment.
it took me a while to realise that getFeaturePropertySetsForFeatures() takes lists of features already filtered by projectId (or not bound to a project), after which my panic for having only a filter by bbox (as it was already) rather than this being a narrowing of an existing set subsided 😅
as for the removed test, I am not visualizing in my mind why this started failing now (I suppose that the filter by bbox was not working correctly in the tested project), but I'd say we should keep a test for this - if not with "real" data (as in, what we seed dbs with in test environments) at least with a minimal mock repository - what do you think?
kgajowy
force-pushed
the
fix/MARXAN-859-search-feature-query
branch
from
727352a
to
2ba5699
Compare
@hotzevzl test restored. Lower amount of results may be happening due to incorrectly applied |
PR is hard to read in one diff - recommended way it commit-by-commit.
First one contains a little re-organization of "AppInfoDTO" thingy to keep known properties really known - during that it proved that some property is "used" but not really created/added at any point, thus leading to not using project id correctly
Second is another fix - previously the
OR like ...was out of the other conditions, totally ignoring project limitations. Thanks @Sikora00 for spotting this!With search param:
